-
Notifications
You must be signed in to change notification settings - Fork 957
Simplify build: add libcommon.a instead of listing all common objects #8594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rustyrussell
wants to merge
9
commits into
ElementsProject:master
Choose a base branch
from
rustyrussell:guilt/simplify-build-libcommon
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Simplify build: add libcommon.a instead of listing all common objects #8594
rustyrussell
wants to merge
9
commits into
ElementsProject:master
from
rustyrussell:guilt/simplify-build-libcommon
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rusty Russell <[email protected]>
This means we don't have to manually choose what to link against, which is much of the complexity of our Makefiles: the compiler will automatically use any object files it needs to link. We already do this for ccan as libccan.a, now we have libcommon.a. We don't link against it for *everything*, as some tests require their own versions. Notes: 1. I get rid of the weird plugins/test/Makefile2 (accidental commit?) 2. Many tests change due to update-mocks. 3. In some places I added the missing dependency on the Makefile itself, though most are in the next patch. Before: Total program size: 221366528 Total tests size: 364243856 After: Total program size: 190733656 Total tests size: 337880888 Build time from make clean (RUST=0) (includes building external libs): Before: real 0m38.227000-44.245000(41.8222+/-1.6)s user 3m2.105000-33.696000(23.1442+/-8.4)s sys 0m35.054000-42.269000(39.7231+/-2)s After: real 0m38.944000-40.416000(40.1131+/-0.4)s user 3m6.790000-17.159000(15.0571+/-2.8)s sys 0m35.304000-37.336000(36.8942+/-0.57)s Build time after touch config.vars (RUST=0): Before: real 0m18.928000-22.776000(21.5084+/-1.1)s user 2m8.613000-36.567000(27.7281+/-7.7)s sys 0m20.458000-23.436000(22.3963+/-0.77)s After: real 0m19.831000-21.862000(21.5528+/-0.58)s user 2m15.361000-30.731000(28.4798+/-4.4)s sys 0m21.056000-22.339000(22.0346+/-0.35)s Signed-off-by: Rusty Russell <[email protected]> rusty@rusty-Framework:~/devel/cvs/lightni
…their Makefile. 1. $(JSMN_OBJS) is not set anywhere. 2. You don't need to depend on CCAN_HEADERS: the top level Makefile has all object depedning on it. 3. Similarly, CCAN_OBJS. 4. Every object file should be rebuilt if its Makefile changes. Signed-off-by: Rusty Russell <[email protected]>
8200465
to
56ca9a5
Compare
Each header should only include the other headers it needs to compile; `devtools/reduce-includes.sh */*.h` does this. The C files then need additional includes if they don't compile. And remove the entirely useless wire/onion_wire.h, which only serves to include wire/onion_wiregen.h. Signed-off-by: Rusty Russell <[email protected]>
…ludes. Even if we would currently include it indirectly, we must include it directly. Signed-off-by: Rusty Russell <[email protected]>
Basically, `devtools/reduce-includes.sh */*.c`. Build time from make clean (RUST=0) (includes building external libs): Before: real 0m38.944000-40.416000(40.1131+/-0.4)s user 3m6.790000-17.159000(15.0571+/-2.8)s sys 0m35.304000-37.336000(36.8942+/-0.57)s After: real 0m37.872000-39.974000(39.5466+/-0.59)s user 3m1.211000-14.968000(12.4556+/-3.9)s sys 0m35.008000-36.830000(36.4143+/-0.5)s Build time after touch config.vars (RUST=0): Before: real 0m19.831000-21.862000(21.5528+/-0.58)s user 2m15.361000-30.731000(28.4798+/-4.4)s sys 0m21.056000-22.339000(22.0346+/-0.35)s After: real 0m18.384000-21.307000(20.8605+/-0.92)s user 2m5.585000-26.843000(23.6017+/-6.7)s sys 0m19.650000-22.003000(21.4943+/-0.69)s Signed-off-by: Rusty Russell <[email protected]>
… for fuzzing. This makes it trivial to run the fuzz tests as unit tests in non-fuzzing mode. Signed-off-by: Rusty Russell <[email protected]>
This happens in the fuzzer corpora, but that doesn't check for take() leaks. Our unit tests do: ``` fuzz-initial_channel: outstanding taken(): 0x626c3b3affc8 make: *** [Makefile:823: unittest/tests/fuzz/fuzz-initial_channel] Error 1 ``` This doesn't matter in real life, since we exit the subdaemon if this fails, but it's still a bug. Signed-off-by: Rusty Russell <[email protected]>
This means we can make sure the compile and run in normal builds. Signed-off-by: Rusty Russell <[email protected]>
56ca9a5
to
e528b59
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main change here is to generate a single archive with all the common, wire and bitcoin objects in it. Programs can simply link against this instead of listing each object file they need: the linker will only use the object files needed. This simplifies Makefiles a great deal, and actually shrinks our binaries a little.
I also took the chance to clean up our Makefiles and reduce our includes.
Changelog-None: build cleanups only.